fix(crypto): Serialize sender data msk in base64 instead of numbers #4450
Conversation
BillCarsonFr
force-pushed
the
valere/add_some_crypto_insta_snapshot_tests
branch
from
01bf4ba to
4e6c060
Compare
BillCarsonFr
force-pushed
the
valere/serialize_known_sender_data_b64
branch
from
5496d77 to
3c94504
Compare
BillCarsonFr
requested review from
Hywan,
poljar and
andybalaam
and removed request for
a team and
Hywan
| pub master_key: Box<Ed25519PublicKey>, | ||
| } | ||
|
|
||
| /// In an initial version the master key was serialized as an array of number, | ||
| /// it is now exported in base64. This code adds backward compatibility. | ||
| pub(crate) fn deserialize_sender_msk_base64_or_array<'de, D>( |
There was a problem hiding this comment.
I am not familiar with this serde visitor pattern. Let me know if it is sane or if there is a prefferred method? I am not sure about the error mappings
|
Filed #4459 about the test failure |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4450 +/- ##
==========================================
- Coverage 85.12% 85.09% -0.04%
==========================================
Files 283 283
Lines 31775 31790 +15
==========================================
+ Hits 27048 27051 +3
- Misses 4727 4739 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This looks good to me, and the error mappings look about right.
@poljar should Edd25519PublicKey serialize as base64 by default?
No, only if we serialize into JSON. |
To expand on this, I think this PR is fine, but we don't want to move the base64 serialization into vodozemac, where |
BillCarsonFr
force-pushed
the
valere/serialize_known_sender_data_b64
branch
from
3c94504 to
5ff556f
Compare
Follow up of #4449
While adding snapshot test, I detected that sender's data
KnownSenderDatawas serializing the cross-signing master key as an array of number. This PR changes that and now serialize the key as a base64 string, adding migration for the old format.Signed-off-by: